Skip to content

feat(emails): cold-start nudge sweep for welcomed accounts with no artist (chat#1889 row 15) - #789

Open
sweetmantech wants to merge 9 commits into
mainfrom
feat/cold-start-nudge-sweep
Open

feat(emails): cold-start nudge sweep for welcomed accounts with no artist (chat#1889 row 15)#789
sweetmantech wants to merge 9 commits into
mainfrom
feat/cold-start-nudge-sweep

Conversation

@sweetmantech

@sweetmantech sweetmantech commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Matrix row 15 in recoupable/chat#1889. The issue had this deferred pending "detection infra (scheduled sweep + dedupe marker)" — this PR builds that infra. Stacked on #784 (shared email layout) and contains #788's commit (schedule-confirmation email) beneath it; review after both.

Why

The welcome email fires on account creation, regardless of whether a valuation preceded it. So a cold-start signup gets an email telling them to "confirm your artists" and "see your baseline valuation" for records that do not exist.

Measured, email_send_log 2026-07-23 → 2026-07-26: of 9 accounts welcomed, 6 had zero artists, zero catalogs, and had never run a valuation.

There is no event to hang detection on — the signal is the absence of a roster some time later — so this is a scheduled sweep rather than a handler-fired send.

What changed

  • getColdStartAccountIds (pure) — welcomed, minus rostered, minus already-nudged. Pure so the selection rule is testable without the sweep's three reads.
  • selectRosteredAccountIds — the account side of account_artist_ids, which the existing selectAccountArtistIds does not return (it selects only artist_id). Fails closed on a read error: it returns the input set, so a database problem can never make every account look cold-start and trigger a mass nudge.
  • selectEmailSendLog gains createdAfter / createdBefore — additive, no existing caller affected.
  • buildColdStartNudgeEmail / sendColdStartNudgeEmail — on the shared renderEmailLayout, asking for the one thing that unblocks everything else (pick the artist), CTA to /setup/artists.
  • runColdStartNudgeSweep + coldStartNudgeHandler behind CRON_SECRET (validateCronRequest, mirroring the two existing internal cron handlers), wired to a daily Vercel cron at 15:00 UTC in vercel.json.

Window: 1 to 14 days after the welcome. Below a day the user may still be mid-setup; past two weeks a nudge reads as spam rather than a reminder. Dedupe is the cold_start_nudge_email marker in email_send_log, so a cron retry or a manual re-run cannot double-send. Sends are sequential on purpose — no latency requirement, and it keeps the send rate gentle.

Verification

TDD, red → green (getColdStartAccountIds RED as "Cannot find module", then green):

```
pnpm exec vitest run lib/onboarding
Test Files 2 passed (2)
Tests 9 passed (9)
```

Sweep tests cover: only-unrostered are nudged, an already-nudged account is skipped (the retry guard), the exact 1-to-14-day window boundaries (createdAfter / createdBefore asserted against an injected now), a no-op empty window that skips the roster read entirely, and sent counting only sends that actually went out (e.g. a wallet-only account with no address).

```
pnpm exec vitest run # full suite, no regressions
Test Files 788 passed (788)
Tests 4266 passed (4266)
```

pnpm exec tsc --noEmit clean for the touched files; vercel.json re-validated as parseable JSON.

Not yet verified live — this needs a cron invocation against the preview with CRON_SECRET to confirm it selects the real cold-start cohort before it is allowed to send to them. I would recommend a dry-run flag or a Telegram-only first pass before this goes to production, since it emails real users.

Tracked in chat#1889 (matrix row 15).


Summary by cubic

Adds a daily cold-start nudge that emails welcomed accounts with no artist to prompt roster setup, fulfilling chat#1889 row 15. Also unifies email styling and confirms newly created schedules so signups know what will arrive and when.

  • New Features
    • Daily Vercel cron (15:00 UTC) calls GET /api/internal/cold-start-nudge, gated by CRON_SECRET.
    • Selection: welcomed 1–14 days ago (from email_send_log), minus rostered (account_artist_ids), minus already nudged; roster read fails closed to prevent mass sends.
    • Dedupe via cold_start_nudge_email in email_send_log; sequential sends; logs each attempt.
    • New email: buildColdStartNudgeEmail + sendColdStartNudgeEmail, shared layout, CTA to /setup/artists.
    • Shared renderEmailLayout adopted by welcome, valuation, and weekly-report paths (via processAndSendEmail).
    • New schedule-confirmation email (sendScheduleConfirmationEmail) sent after task creation; cadence from describeCronCadence; deduped per task in email_send_log.
    • Infra: selectEmailSendLog adds createdAfter/createdBefore; new selectRosteredAccountIds; cron wired in vercel.json.

Written for commit 315050a. Summary will update on new commits.

Review in cubic

sweetmantech and others added 6 commits July 23, 2026 06:28
Welcome, valuation, and weekly-report emails should share one visual language
(DESIGN.md — four-font system, shadow-as-border, achromatic chrome), but each
outbound email was assembled ad hoc (body + bare footer). This establishes the
shared template (recoupable/chat#1885 consistency pass).

- `renderEmailLayout` (`lib/emails/`) — one header + footer + optional-CTA
  wrapper: Recoup wordmark header, achromatic shadow-as-border card, the
  DESIGN.md font stack, centered fixed-max-width container. Email-client-safe
  (inline styles, literal hex tokens, webfonts degrade to system fonts).
- `processAndSendEmail` — adopts the wrapper as the proof point. This is the
  path the live weekly-report email flows through (agent `send_email` →
  `processAndSendEmail`), so the already-live weekly report now renders in the
  shared house style, with the existing footer carried in as the layout footer.

Welcome (api#774) and valuation (api#773) emails adopt the same wrapper once
merged — this PR establishes the shared template.

TDD: RED→GREEN for the layout renderer (body/footer/CTA + house-style markers)
and for the processAndSendEmail adoption.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t-stack quoting

Completes the consistency pass — previously the wrapper was only adopted in the
weekly-report path, so welcome/valuation still shipped their own chrome.

- buildWelcomeEmail + renderValuationReportHtml now render through
  renderEmailLayout (body + cta + footer), dropping their duplicated outer
  page/card chrome; each keeps only its own content.
- Fix: FONT_STACK used double-quoted font names inside double-quoted
  style="…" attributes, which terminated the attribute early — silently
  dropping the font (clients fell back to serif) AND every declaration after
  it (the CTA button lost its background/padding/radius). Single-quote the
  font names; add a regression test.
- Layout footer tagline: em dash -> comma (house copy rule).
- Consistency assertions added to both email tests.

199 lib/emails tests pass; lint + format clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chat#1889 matrix row 14 — the last genuine gap in the email journey.

A signup finished onboarding and then heard nothing until a report arrived days
later, with no record that anything had actually been scheduled.

- New buildScheduleConfirmationEmail renders through the shared renderEmailLayout
  (api#784), so it reads as one family with welcome/valuation/weekly-report.
- New describeCronCadence turns the cron into "Mondays at 13:00 UTC",
  deliberately narrow: it only describes the fixed weekday/daily shapes
  onboarding creates and otherwise falls back to the raw expression, since an
  honest cron string beats a confidently wrong sentence about delivery time.
- New sendScheduleConfirmationEmail fires from createTaskHandler after the
  schedule is materialized, deduped per task id in email_send_log, and swallows
  its own failures so it can never fail task creation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tist

chat#1889 matrix row 15 — was deferred pending detection infra; this builds it.

The welcome email fires on account creation whether or not a valuation preceded
it, so a cold-start signup is told to confirm a roster and view a valuation that
does not exist. Six of the nine accounts welcomed 2026-07-23..25 had zero
artists. There is no event to hang detection on, because the signal is the
ABSENCE of a roster some time later, so this is a scheduled sweep.

- getColdStartAccountIds (pure): welcomed, minus rostered, minus already nudged.
- selectRosteredAccountIds: the account side of account_artist_ids, which the
  existing selector does not return. Fails CLOSED on a read error so a database
  problem can never make every account look cold-start and mass-nudge.
- selectEmailSendLog gains createdAfter/createdBefore (additive).
- buildColdStartNudgeEmail / sendColdStartNudgeEmail on the shared layout.
- runColdStartNudgeSweep + coldStartNudgeHandler behind CRON_SECRET, wired to a
  daily Vercel cron at 15:00 UTC.

Window is 1 to 14 days after the welcome: below a day the user may still be
mid-setup, past two weeks the nudge reads as spam. Dedupe is the
cold_start_nudge_email marker in email_send_log, so a cron retry cannot
double-send.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 27, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@vercel

vercel Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
api Ready Ready Preview Jul 27, 2026 2:22pm

Request Review

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@sweetmantech, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 56 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b6137116-f2e3-47bc-b9f6-4c6ccc2c0a44

📥 Commits

Reviewing files that changed from the base of the PR and between 96959eb and 315050a.

⛔ Files ignored due to path filters (10)
  • lib/emails/__tests__/buildScheduleConfirmationEmail.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/emails/__tests__/buildWelcomeEmail.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/emails/__tests__/describeCronCadence.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/emails/__tests__/processAndSendEmail.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/emails/__tests__/renderEmailLayout.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/emails/valuationReport/__tests__/renderValuationReportHtml.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/onboarding/__tests__/getColdStartAccountIds.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/onboarding/__tests__/runColdStartNudgeSweep.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/tasks/__tests__/createTaskHandler.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • vercel.json is excluded by none and included by none
📒 Files selected for processing (17)
  • app/api/internal/cold-start-nudge/route.ts
  • lib/const.ts
  • lib/emails/buildColdStartNudgeEmail.ts
  • lib/emails/buildScheduleConfirmationEmail.ts
  • lib/emails/buildWelcomeEmail.ts
  • lib/emails/describeCronCadence.ts
  • lib/emails/processAndSendEmail.ts
  • lib/emails/renderEmailLayout.ts
  • lib/emails/sendColdStartNudgeEmail.ts
  • lib/emails/sendScheduleConfirmationEmail.ts
  • lib/emails/valuationReport/renderValuationReportHtml.ts
  • lib/onboarding/coldStartNudgeHandler.ts
  • lib/onboarding/getColdStartAccountIds.ts
  • lib/onboarding/runColdStartNudgeSweep.ts
  • lib/supabase/account_artist_ids/selectRosteredAccountIds.ts
  • lib/supabase/email_send_log/selectEmailSendLog.ts
  • lib/tasks/createTaskHandler.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/cold-start-nudge-sweep

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 issues found across 27 files

Confidence score: 2/5

  • In lib/onboarding/coldStartNudgeHandler.ts, returning error.message in HTTP responses can expose internal details that other cron handlers currently avoid, creating an avoidable information-leak path — return a generic error body and keep specifics in server logs only.
  • lib/onboarding/runColdStartNudgeSweep.ts has multiple idempotency gaps (racey read-before-send and treating marker-read failures as empty state), and lib/emails/sendColdStartNudgeEmail.ts can lose the only dedupe record when log writes fail, so users can receive duplicate nudges — add an atomic per-account reservation/unique key before send and fail closed when prior-marker reads fail.
  • lib/emails/sendScheduleConfirmationEmail.ts uses a non-atomic check-then-log flow, so concurrent retries can issue duplicate confirmations to the same task — reserve send intent with a unique idempotency key before calling Resend.
  • lib/tasks/createTaskHandler.ts now blocks success on downstream email/DB calls, so slow dependencies can make already-created schedules look failed and trigger duplicate client retries; also lib/emails/describeCronCadence.ts misses Sunday value 7, degrading confirmation clarity — move email work off the request critical path and normalize 0/7 Sunday handling.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="lib/emails/renderEmailLayout.ts">

<violation number="1" location="lib/emails/renderEmailLayout.ts:17">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The comments in `renderEmailLayout.ts` reference `DESIGN.md §3` and `DESIGN.md four-font system`, but `DESIGN.md` does not exist in the repository. Citing a non-existent document misleads future maintainers and is an example of a fabricated reference that Rule 4 specifically flags. Remove the `DESIGN.md` citations or create the file with the cited sections.</violation>
</file>

<file name="lib/onboarding/runColdStartNudgeSweep.ts">

<violation number="1" location="lib/onboarding/runColdStartNudgeSweep.ts:60">
P1: A transient `email_send_log` read failure turns this dedupe set empty and sends a nudge to accounts already nudged. Treat failure to load prior markers as a failed/no-op sweep rather than as no markers.</violation>

<violation number="2" location="lib/onboarding/runColdStartNudgeSweep.ts:76">
P1: Overlapping sweep invocations can send this nudge twice: both pass the read-before-send check before either inserts its marker. Reserve a per-account nudge marker atomically before calling Resend (with a DB uniqueness constraint/claim), then send only for the invocation that acquired it.</violation>
</file>

<file name="lib/emails/sendColdStartNudgeEmail.ts">

<violation number="1" location="lib/emails/sendColdStartNudgeEmail.ts:48">
P1: A successful delivery can be nudged again after an `email_send_log` write failure, because `logEmailAttempt` swallows that failure while the sweep's only idempotency record is this `sent` row. Consider durable claim/idempotency handling before sending so retries cannot issue another Resend request when recording the first send fails.</violation>
</file>

<file name="lib/emails/sendScheduleConfirmationEmail.ts">

<violation number="1" location="lib/emails/sendScheduleConfirmationEmail.ts:40">
P2: Concurrent retries can send duplicate schedule confirmations: this check and the later log write do not reserve the task before the Resend call. Use an atomic, uniquely constrained send reservation/idempotency key before sending, then update it with the result.</violation>
</file>

<file name="lib/tasks/createTaskHandler.ts">

<violation number="1" location="lib/tasks/createTaskHandler.ts:35">
P2: Task creation now waits for several email/DB network calls before returning success. A slow or stalled email dependency can make an already-created schedule appear failed to the client and encourage duplicate task retries; enqueue the confirmation outside this request lifecycle (or otherwise bound it) instead of awaiting delivery here.</violation>
</file>

<file name="lib/emails/describeCronCadence.ts">

<violation number="1" location="lib/emails/describeCronCadence.ts:48">
P3: Sunday schedules written with the valid `7` cron value render as raw cron text instead of a Sunday cadence. Treat `7` as the same Sunday index as `0` so confirmations remain human-readable for both supported forms.</violation>
</file>

<file name="lib/onboarding/coldStartNudgeHandler.ts">

<violation number="1" location="lib/onboarding/coldStartNudgeHandler.ts:32">
P1: The error response leaks `error.message` into the HTTP response body when the caught error is an `Error` instance. Both existing cron handlers (`getCreditSpendDigestHandler` and `playcountMaintenanceHandler`) always return a hardcoded string ("Internal server error" / "Internal error") and never include the raw exception text in the response. Leaking error messages can expose stack traces, DB column names, file paths, or other internal details. Log the full error server-side (already done), but return a hardcoded message to the caller.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant Cron as Vercel Cron
    participant API as API Route
    participant Auth as validateCronRequest
    participant Sweep as runColdStartNudgeSweep
    participant SelectLog as selectEmailSendLog
    participant SelectRoster as selectRosteredAccountIds
    participant SelectCold as getColdStartAccountIds
    participant SendEmail as sendColdStartNudgeEmail
    participant SelectEmails as selectAccountEmails
    participant Resend as Resend API
    participant Log as logEmailAttempt
    participant DB as Database

    Note over Cron,API: Daily cron triggered at 15:00 UTC

    Cron->>API: GET /api/internal/cold-start-nudge
    API->>Auth: validateCronRequest(CRON_SECRET)
    alt Invalid CRON_SECRET
        Auth-->>API: 403 denied
        API-->>Cron: 403 Forbidden
    end

    Note over Sweep,SelectLog: Determine eligible accounts

    Auth-->>API: valid
    API->>Sweep: runColdStartNudgeSweep(now)
    Sweep->>SelectLog: selectEmailSendLog(status="sent", rawBodyLike=welcome marker, createdAfter=14d ago, createdBefore=1d ago)
    SelectLog->>DB: Query email_send_log
    DB-->>SelectLog: Welcome rows
    SelectLog-->>Sweep: welcomedAccountIds (e.g. ["a","b","c"])

    alt No welcomed accounts in window
        Sweep-->>API: {welcomed:0, coldStart:0, sent:0}
        API-->>Cron: 200 OK (empty sweep)
    else Has welcomed accounts
        Sweep->>SelectRoster: selectRosteredAccountIds(["a","b","c"])
        SelectRoster->>DB: Query account_artist_ids WHERE account_id IN [...]
        alt DB error
            DB-->>SelectRoster: error
            Note over SelectRoster: Fails closed - returns input set
            SelectRoster-->>Sweep: ["a","b","c"] (no risk of mass nudge)
        else Success
            DB-->>SelectRoster: ["b"] (rostered accounts)
            SelectRoster-->>Sweep: ["b"]
        end

        Sweep->>SelectLog: selectEmailSendLog(rawBodyLike=cold_start_nudge marker)
        SelectLog->>DB: Query email_send_log
        DB-->>SelectLog: Already-nudged rows
        SelectLog-->>Sweep: alreadyNudgedAccountIds (e.g. ["c"])

        Sweep->>SelectCold: getColdStartAccountIds(welcomed=["a","b","c"], rostered=["b"], nudged=["c"])
        SelectCold-->>Sweep: ["a"] (pure selection logic)

        Note over Sweep,SendEmail: Sequential sends (no concurrency)

        loop For each cold-start account (["a"])
            Sweep->>SendEmail: sendColdStartNudgeEmail(accountId="a")
            SendEmail->>SelectEmails: selectAccountEmails(accountIds="a")
            SelectEmails->>DB: Query account_emails
            DB-->>SelectEmails: [{email: "user@example.com"}]
            SelectEmails-->>SendEmail: Account email

            alt No email found (e.g., wallet-only account)
                SendEmail-->>Sweep: false (send skipped)
            else Email found
                SendEmail->>Resend: sendEmailWithResend(from, to, subject, html)
                alt Resend failure
                    Resend-->>SendEmail: NextResponse (error)
                    SendEmail->>Log: logEmailAttempt(rawBody, status="send_failed", accountId)
                    Log->>DB: Insert email_send_log row
                    Log-->>SendEmail: logged
                    SendEmail-->>Sweep: false
                else Resend success
                    Resend-->>SendEmail: {id: "email-123"}
                    SendEmail->>Log: logEmailAttempt(rawBody, status="sent", accountId, resendId)
                    Log->>DB: Insert email_send_log row (cold_start_nudge_email marker)
                    Log-->>SendEmail: logged
                    SendEmail-->>Sweep: true (send counted)
                end
            end
        end

        Sweep-->>API: {welcomed:3, coldStart:1, sent:1}
        API-->>Cron: 200 OK with result summary
    end
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

for (const accountId of coldStart) {
// Sequential on purpose: a nudge sweep has no latency requirement, and this
// keeps the send rate gentle.
if (await sendColdStartNudgeEmail({ accountId })) sent += 1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Overlapping sweep invocations can send this nudge twice: both pass the read-before-send check before either inserts its marker. Reserve a per-account nudge marker atomically before calling Resend (with a DB uniqueness constraint/claim), then send only for the invocation that acquired it.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/onboarding/runColdStartNudgeSweep.ts, line 76:

<comment>Overlapping sweep invocations can send this nudge twice: both pass the read-before-send check before either inserts its marker. Reserve a per-account nudge marker atomically before calling Resend (with a DB uniqueness constraint/claim), then send only for the invocation that acquired it.</comment>

<file context>
@@ -0,0 +1,84 @@
+  for (const accountId of coldStart) {
+    // Sequential on purpose: a nudge sweep has no latency requirement, and this
+    // keeps the send rate gentle.
+    if (await sendColdStartNudgeEmail({ accountId })) sent += 1;
+  }
+
</file context>

selectRosteredAccountIds(welcomedAccountIds),
selectEmailSendLog({
status: "sent",
rawBodyLike: `"type":"${COLD_START_NUDGE_EMAIL_LOG_TYPE}"`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: A transient email_send_log read failure turns this dedupe set empty and sends a nudge to accounts already nudged. Treat failure to load prior markers as a failed/no-op sweep rather than as no markers.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/onboarding/runColdStartNudgeSweep.ts, line 60:

<comment>A transient `email_send_log` read failure turns this dedupe set empty and sends a nudge to accounts already nudged. Treat failure to load prior markers as a failed/no-op sweep rather than as no markers.</comment>

<file context>
@@ -0,0 +1,84 @@
+    selectRosteredAccountIds(welcomedAccountIds),
+    selectEmailSendLog({
+      status: "sent",
+      rawBodyLike: `"type":"${COLD_START_NUDGE_EMAIL_LOG_TYPE}"`,
+    }),
+  ]);
</file context>

return false;
}

await logEmailAttempt({

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: A successful delivery can be nudged again after an email_send_log write failure, because logEmailAttempt swallows that failure while the sweep's only idempotency record is this sent row. Consider durable claim/idempotency handling before sending so retries cannot issue another Resend request when recording the first send fails.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/emails/sendColdStartNudgeEmail.ts, line 48:

<comment>A successful delivery can be nudged again after an `email_send_log` write failure, because `logEmailAttempt` swallows that failure while the sweep's only idempotency record is this `sent` row. Consider durable claim/idempotency handling before sending so retries cannot issue another Resend request when recording the first send fails.</comment>

<file context>
@@ -0,0 +1,59 @@
+      return false;
+    }
+
+    await logEmailAttempt({
+      rawBody,
+      status: "sent",
</file context>

return NextResponse.json(
{
status: "error",
error: error instanceof Error ? error.message : "Internal server error",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: The error response leaks error.message into the HTTP response body when the caught error is an Error instance. Both existing cron handlers (getCreditSpendDigestHandler and playcountMaintenanceHandler) always return a hardcoded string ("Internal server error" / "Internal error") and never include the raw exception text in the response. Leaking error messages can expose stack traces, DB column names, file paths, or other internal details. Log the full error server-side (already done), but return a hardcoded message to the caller.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/onboarding/coldStartNudgeHandler.ts, line 32:

<comment>The error response leaks `error.message` into the HTTP response body when the caught error is an `Error` instance. Both existing cron handlers (`getCreditSpendDigestHandler` and `playcountMaintenanceHandler`) always return a hardcoded string ("Internal server error" / "Internal error") and never include the raw exception text in the response. Leaking error messages can expose stack traces, DB column names, file paths, or other internal details. Log the full error server-side (already done), but return a hardcoded message to the caller.</comment>

<file context>
@@ -0,0 +1,37 @@
+    return NextResponse.json(
+      {
+        status: "error",
+        error: error instanceof Error ? error.message : "Internal server error",
+      },
+      { status: 500, headers: getCorsHeaders() },
</file context>

cta?: EmailLayoutCta;
};

// Brand tokens (DESIGN.md §3) — kept literal because email clients can't read

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Custom agent: Flag AI Slop and Fabricated Changes

The comments in renderEmailLayout.ts reference DESIGN.md §3 and DESIGN.md four-font system, but DESIGN.md does not exist in the repository. Citing a non-existent document misleads future maintainers and is an example of a fabricated reference that Rule 4 specifically flags. Remove the DESIGN.md citations or create the file with the cited sections.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/emails/renderEmailLayout.ts, line 17:

<comment>The comments in `renderEmailLayout.ts` reference `DESIGN.md §3` and `DESIGN.md four-font system`, but `DESIGN.md` does not exist in the repository. Citing a non-existent document misleads future maintainers and is an example of a fabricated reference that Rule 4 specifically flags. Remove the `DESIGN.md` citations or create the file with the cited sections.</comment>

<file context>
@@ -0,0 +1,88 @@
+  cta?: EmailLayoutCta;
+};
+
+// Brand tokens (DESIGN.md §3) — kept literal because email clients can't read
+// CSS custom properties. Achromatic chrome; color comes from content.
+const INK = "#0a0a0a"; // --foreground
</file context>

}): Promise<void> {
try {
const marker = `"task_id":"${taskId}"`;
const alreadySent = await selectEmailSendLog({

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Concurrent retries can send duplicate schedule confirmations: this check and the later log write do not reserve the task before the Resend call. Use an atomic, uniquely constrained send reservation/idempotency key before sending, then update it with the result.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/emails/sendScheduleConfirmationEmail.ts, line 40:

<comment>Concurrent retries can send duplicate schedule confirmations: this check and the later log write do not reserve the task before the Resend call. Use an atomic, uniquely constrained send reservation/idempotency key before sending, then update it with the result.</comment>

<file context>
@@ -0,0 +1,90 @@
+}): Promise<void> {
+  try {
+    const marker = `"task_id":"${taskId}"`;
+    const alreadySent = await selectEmailSendLog({
+      accountId,
+      status: "sent",
</file context>

// Bridge from signing up to the first report landing (chat#1889): confirm
// what was scheduled and when. Fires after the schedule is materialized,
// and is best-effort inside, so it can never fail task creation.
await sendScheduleConfirmationEmail({

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Task creation now waits for several email/DB network calls before returning success. A slow or stalled email dependency can make an already-created schedule appear failed to the client and encourage duplicate task retries; enqueue the confirmation outside this request lifecycle (or otherwise bound it) instead of awaiting delivery here.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/tasks/createTaskHandler.ts, line 35:

<comment>Task creation now waits for several email/DB network calls before returning success. A slow or stalled email dependency can make an already-created schedule appear failed to the client and encourage duplicate task retries; enqueue the confirmation outside this request lifecycle (or otherwise bound it) instead of awaiting delivery here.</comment>

<file context>
@@ -28,6 +29,17 @@ export async function createTaskHandler(request: NextRequest): Promise<NextRespo
+    // Bridge from signing up to the first report landing (chat#1889): confirm
+    // what was scheduled and when. Fires after the schedule is materialized,
+    // and is best-effort inside, so it can never fail task creation.
+    await sendScheduleConfirmationEmail({
+      accountId: validatedBody.account_id,
+      taskId: createdTask.id,
</file context>


if (dayOfWeek === "*") return `every day at ${at}`;

if (/^[0-6]$/.test(dayOfWeek)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Sunday schedules written with the valid 7 cron value render as raw cron text instead of a Sunday cadence. Treat 7 as the same Sunday index as 0 so confirmations remain human-readable for both supported forms.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/emails/describeCronCadence.ts, line 48:

<comment>Sunday schedules written with the valid `7` cron value render as raw cron text instead of a Sunday cadence. Treat `7` as the same Sunday index as `0` so confirmations remain human-readable for both supported forms.</comment>

<file context>
@@ -0,0 +1,53 @@
+
+  if (dayOfWeek === "*") return `every day at ${at}`;
+
+  if (/^[0-6]$/.test(dayOfWeek)) {
+    return `${WEEKDAYS[Number(dayOfWeek)]} at ${at}`;
+  }
</file context>

sweetmantech and others added 3 commits July 27, 2026 09:18
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 5 files (changes from recent commits).

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Requires human review: Auto-approval blocked by 8 unresolved issues from previous reviews.

Re-trigger cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant